-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use GenServer instead of Task for the default strategy process #36
Conversation
… setting up only one module for one strategy, and not useable for dynamic spinning up of strategies
This is awesome work @lovebes ! Thanks for your contribution. I should've done this a long time ago. Just some minor nitpicks with the pipes for opts. I am interested in having another strategy that spans strategies dynamically. I am also planning on adding more default validations like x5t and so on. If you want to hack on those please do :) |
Co-authored-by: Victor Oliveira Nascimento <[email protected]>
@victorolinasc I've applied your changes. Oh awesome! Yes I would very much like to try my hand in creating the strategy that creates other strategies dynamically. |
@victorolinasc I started working on the dynamic supervisor route, and have realized what more to change in DefaultStrategyTemplate to make it work for both "standalone" and in dynamic supervisor mode. I'll add the changes in this PR, or in another PR if that's better. |
Alright I'm done, I actually changed a lot more and made it useable for when it is needed to be run for dynamically starting strategies. #39 will include and expand this PR. |
Polling is a bit more idiomatic when done inside GenServer.
Also, having GenServer sets the setting for dynamic spinning up of strategy modules.
As-is, it can't, because EtsCache needs to have dynamic table naming too, which needs more customization - at that point it should be done in a seperate, custom strategy (probably involving a Registry too).
Therefore this is still "locked" to spin up pre-defined strategy modules.
In addition, I changed the API calls to be synchronous, as you'd still want to process the outcome of the current API call response before you schedule another API call to prevent any possibility of firing the next API call even when processing the current one didn't finish yet.